USHIFT-6836: Bump cert manager to 19.0 with trust-manager support#6552
Open
eslutsky wants to merge 5 commits intoopenshift:mainfrom
Open
USHIFT-6836: Bump cert manager to 19.0 with trust-manager support#6552eslutsky wants to merge 5 commits intoopenshift:mainfrom
eslutsky wants to merge 5 commits intoopenshift:mainfrom
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBumps cert-manager to v1.19.x: updates multiple CRD metadata/schemas, adds TrustManager and Bundle CRDs, adjusts list semantics and printer columns, extends RBAC, updates images/releases, adds trusted CA ConfigMap, and updates kustomize/packaging and auto-rebase assets. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eslutsky The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@assets/optional/cert-manager/manager/manager.yaml`:
- Around line 87-96: The RELATED_IMAGE_CERT_MANAGER_TRUST_MANAGER is hardcoded
in manager.yaml and not included in the cert-manager image pipeline, so add
trust-manager to the same release metadata/configmap used for other operands
(e.g., cert-manager-images) and update manager.yaml to reference the
pinned/mirrored entry instead of the Quay tag; specifically, add a trust-manager
image key/value into the cert-manager-images ConfigMap and the per-arch image
lists (assets/optional/cert-manager/manager/images-x86_64.yaml,
images-aarch64.yaml) and release metadata (release-cert-manager-aarch64.json) so
the image pipeline generates a RELATED_IMAGE_CERT_MANAGER_TRUST_MANAGER entry
that manager.yaml can consume, ensuring disconnected installs and digest pinning
work as with the other RELATED_IMAGE_* variables.
In `@assets/optional/cert-manager/manager/trusted_ca_bundle_configmap.yaml`:
- Around line 1-7: Add the new ConfigMap manifest to the manager component
kustomization resources and set the environment variable
TRUSTED_CA_CONFIGMAP_NAME in the manager Deployment to "trusted-ca-bundle";
specifically, add the
assets/optional/cert-manager/manager/trusted_ca_bundle_configmap.yaml entry to
the resources list in assets/optional/cert-manager/manager/kustomization.yaml
and update the manager Deployment/PodSpec env var TRUSTED_CA_CONFIGMAP_NAME in
manager.yaml (the container spec that sets this env) to the literal value
trusted-ca-bundle so the operator receives the correct ConfigMap name for
trusted CA bundle injection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fd9df91-5d4f-4d4d-b0e1-bace9cc0ac48
📒 Files selected for processing (18)
assets/optional/cert-manager/crd/bases/certificaterequests.cert-manager.io-crd.yamlassets/optional/cert-manager/crd/bases/certificates.cert-manager.io-crd.yamlassets/optional/cert-manager/crd/bases/challenges.acme.cert-manager.io-crd.yamlassets/optional/cert-manager/crd/bases/clusterissuers.cert-manager.io-crd.yamlassets/optional/cert-manager/crd/bases/issuers.cert-manager.io-crd.yamlassets/optional/cert-manager/crd/bases/operator.openshift.io_certmanagers.yamlassets/optional/cert-manager/crd/bases/operator.openshift.io_istiocsrs.yamlassets/optional/cert-manager/crd/bases/orders.acme.cert-manager.io-crd.yamlassets/optional/cert-manager/crd/kustomization.yamlassets/optional/cert-manager/manager/images-aarch64.yamlassets/optional/cert-manager/manager/images-x86_64.yamlassets/optional/cert-manager/manager/manager.yamlassets/optional/cert-manager/manager/trusted_ca_bundle_configmap.yamlassets/optional/cert-manager/rbac/kustomization.yamlassets/optional/cert-manager/rbac/role.yamlassets/optional/cert-manager/release-cert-manager-aarch64.jsonassets/optional/cert-manager/release-cert-manager-x86_64.jsonscripts/auto-rebase/last_rebase_cert_manager.sh
openshift-ci
Bot
added
the
needs-rebase
eslutsky
force-pushed
the
bump-cert-manager-to-19.0
branch
from
e6eb3b8 to
cbea015
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.yml`:
- Around line 33-37: The CRD condition type is inconsistent: the description and
documented known condition use "Bundle" while the jsonPath entries use "Synced";
update the jsonPath/name/type/description entries under the Bundle condition
block so they consistently reference the same condition type (either rename the
jsonPath filter and name from "Synced" to "Bundle" or update the documented
known condition to "Synced"), e.g. make the jsonPath .status.conditions[?(@.type
== "Bundle")].status and .reason match the "Bundle" description; check and
update all occurrences of "Synced" and "Bundle" in this CRD (including the
jsonPath for .status.conditions[*].status and .reason) so they are aligned.
In
`@assets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yaml`:
- Around line 1210-1224: The trustNamespace field currently only validates
length and immutability; add a DNS-1123 label pattern check so invalid namespace
names (e.g., starting with digits or containing underscores) are rejected early.
Update the trustNamespace schema to include an x-kubernetes-validations or
pattern entry enforcing the regex ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$ (or
equivalent) and adjust the description to mention it must follow DNS-1123 label
rules; reference the trustNamespace property and its existing
x-kubernetes-validations block when applying this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a88cb37-5974-4895-b2a0-41167b0d3e61
📒 Files selected for processing (3)
assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.ymlassets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yamlscripts/auto-rebase/assets_cert_manager.yaml
✅ Files skipped from review due to trivial changes (1)
- scripts/auto-rebase/assets_cert_manager.yaml
Comment on lines
+33
to
+37
| jsonPath: .status.conditions[?(@.type == "Synced")].status | ||
| name: Synced | ||
| type: string | ||
| - description: Reason Bundle has Synced status | ||
| jsonPath: .status.conditions[?(@.type == "Synced")].reason |
There was a problem hiding this comment.
Condition type text conflicts with actual condition key.
Line 405 documents Bundle as the known condition type, but Line 33 and Line 37 use Synced. One of these is stale and should be aligned.
Suggested doc fix
- Known condition types are `Bundle`.
+ Known condition types are `Synced`.Also applies to: 403-406
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.yml`
around lines 33 - 37, The CRD condition type is inconsistent: the description
and documented known condition use "Bundle" while the jsonPath entries use
"Synced"; update the jsonPath/name/type/description entries under the Bundle
condition block so they consistently reference the same condition type (either
rename the jsonPath filter and name from "Synced" to "Bundle" or update the
documented known condition to "Synced"), e.g. make the jsonPath
.status.conditions[?(@.type == "Bundle")].status and .reason match the "Bundle"
description; check and update all occurrences of "Synced" and "Bundle" in this
CRD (including the jsonPath for .status.conditions[*].status and .reason) so
they are aligned.
cbea015 to
780e7af
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
assets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yaml (1)
1210-1223:⚠️ Potential issue | 🟡 MinorValidate
trustNamespaceas a real Kubernetes namespace name.Length + immutability still allow invalid values like uppercase or underscores, so bad configs can get admitted and only fail later during reconciliation. Add a DNS-1123 label pattern here.
Suggested schema hardening
trustNamespace: default: cert-manager description: |- trustNamespace is the namespace where trust-manager looks for trust sources (ConfigMaps and Secrets containing CA certificates). Defaults to "cert-manager" if not specified. This field is immutable once set. + This field must be a valid DNS-1123 label. This field can have a maximum of 63 characters. maxLength: 63 minLength: 1 + pattern: '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$' type: string x-kubernetes-validations: - message: trustNamespace is immutable once set rule: oldSelf == '' || self == oldSelf🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yaml` around lines 1210 - 1223, The trustNamespace schema currently enforces length and immutability but not DNS-1123 label format, so add a pattern validation for the trustNamespace field to ensure it matches Kubernetes namespace rules (DNS-1123 label: lowercase alphanumeric and '-', start/end with alphanumeric). Update the trustNamespace schema (the trustNamespace property under operator.openshift.io_trustmanagers.yaml) by adding a pattern key with the DNS-1123 label regex and/or an additional x-kubernetes-validations entry that enforces the regex (augmenting the existing x-kubernetes-validations array rather than replacing it) so invalid values with uppercase letters or underscores are rejected at admission time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@assets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yaml`:
- Around line 1210-1223: The trustNamespace schema currently enforces length and
immutability but not DNS-1123 label format, so add a pattern validation for the
trustNamespace field to ensure it matches Kubernetes namespace rules (DNS-1123
label: lowercase alphanumeric and '-', start/end with alphanumeric). Update the
trustNamespace schema (the trustNamespace property under
operator.openshift.io_trustmanagers.yaml) by adding a pattern key with the
DNS-1123 label regex and/or an additional x-kubernetes-validations entry that
enforces the regex (augmenting the existing x-kubernetes-validations array
rather than replacing it) so invalid values with uppercase letters or
underscores are rejected at admission time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 258f0bd8-09a6-4d4e-8cdb-4bb37781a61d
📒 Files selected for processing (4)
assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.ymlassets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yamlpackaging/rpm/microshift.specscripts/auto-rebase/assets_cert_manager.yaml
✅ Files skipped from review due to trivial changes (1)
- scripts/auto-rebase/assets_cert_manager.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.yml
eslutsky
force-pushed
the
bump-cert-manager-to-19.0
branch
from
780e7af to
394dddb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
assets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yaml (1)
1210-1223:⚠️ Potential issue | 🟡 MinorValidate
trustNamespaceas a namespace name.Length + immutability still admit invalid values like
bad_name, which won't be usable as a Kubernetes namespace. Add a DNS-1123 label pattern here so bad configs are rejected at admission time.Suggested schema hardening
trustNamespace: default: cert-manager description: |- trustNamespace is the namespace where trust-manager looks for trust sources (ConfigMaps and Secrets containing CA certificates). Defaults to "cert-manager" if not specified. This field is immutable once set. This field can have a maximum of 63 characters. + pattern: '^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$' maxLength: 63 minLength: 1 type: stringWhat is the Kubernetes namespace naming rule (DNS-1123 label) and the canonical regex used to validate namespace names?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yaml` around lines 1210 - 1223, The trustNamespace schema currently only enforces length and immutability but not the DNS-1123 label format; update the trustNamespace property in the CRD (the trustNamespace schema block) to include the Kubernetes namespace/DNS-1123 label regex (canonical: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$) as a pattern constraint (keep maxLength: 63 and minLength: 1) so invalid names like "bad_name" are rejected at admission time, and optionally clarify the description to mention the DNS-1123 label requirement.assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.yml (1)
32-37:⚠️ Potential issue | 🟡 MinorMake the condition type consistent.
The printer columns filter on
Synced, but the status schema says the known condition type isBundle. One of these is stale; if the controller emits the other value, thekubectl getcolumns stay empty.Also applies to: 403-405
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.yml` around lines 32 - 37, The condition type is inconsistent between the printerColumns JSONPaths (which filter for .status.conditions[?(@.type == "Synced")] and the CRD status schema (which lists a known condition type "Bundle"); pick one canonical condition type and make both places match: update the status.conditions knownTypes entry (or the printerColumn jsonPath) so the condition name (the "name" field and the jsonPath filter) and the schema's condition type string are identical (e.g., change "Bundle" → "Synced" everywhere or vice versa) including the related status.reason jsonPath entries referenced in the diff and the other occurrences noted (the other similar block around the 403-405 region).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.yml`:
- Around line 222-223: The documentation text for the Bundle status field
references a non-existent field name: change any occurrences of
"defaultCAPackageVersion" in the Bundle description to "defaultCAVersion" so it
matches the schema (.status.defaultCAVersion); update the two places called out
(around the current snippet and the other occurrence at lines ~463-468) to use
the exact symbol "defaultCAVersion" and ensure any explanatory text refers to
the Bundle's status field as ".status.defaultCAVersion".
- Around line 80-97: The schema currently allows invalid Bundle specs because it
doesn't enforce the documented mutual exclusions; update the CRD schema in
customresourcedefinition_bundles.trust.cert-manager.io.yml to add OpenAPI
validation for both source blocks (the main source and the secret source) that:
(1) disallow includeAllKeys and key being set together (make them mutually
exclusive), and (2) disallow name and selector being set together (make them
mutually exclusive); implement this by adding appropriate anyOf/oneOf validation
clauses (or an xor-style anyOf with required lists) referencing the
includeAllKeys, key, name and selector properties so invalid combinations are
rejected at admission for Bundle objects.
---
Duplicate comments:
In
`@assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.yml`:
- Around line 32-37: The condition type is inconsistent between the
printerColumns JSONPaths (which filter for .status.conditions[?(@.type ==
"Synced")] and the CRD status schema (which lists a known condition type
"Bundle"); pick one canonical condition type and make both places match: update
the status.conditions knownTypes entry (or the printerColumn jsonPath) so the
condition name (the "name" field and the jsonPath filter) and the schema's
condition type string are identical (e.g., change "Bundle" → "Synced" everywhere
or vice versa) including the related status.reason jsonPath entries referenced
in the diff and the other occurrences noted (the other similar block around the
403-405 region).
In
`@assets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yaml`:
- Around line 1210-1223: The trustNamespace schema currently only enforces
length and immutability but not the DNS-1123 label format; update the
trustNamespace property in the CRD (the trustNamespace schema block) to include
the Kubernetes namespace/DNS-1123 label regex (canonical:
^[a-z0-9]([-a-z0-9]*[a-z0-9])?$) as a pattern constraint (keep maxLength: 63 and
minLength: 1) so invalid names like "bad_name" are rejected at admission time,
and optionally clarify the description to mention the DNS-1123 label
requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 942161a2-034e-4e83-abe2-22dfd4897153
📒 Files selected for processing (4)
assets/optional/cert-manager/crd/bases/customresourcedefinition_bundles.trust.cert-manager.io.ymlassets/optional/cert-manager/crd/bases/operator.openshift.io_trustmanagers.yamlpackaging/rpm/microshift.specscripts/auto-rebase/assets_cert_manager.yaml
✅ Files skipped from review due to trivial changes (1)
- packaging/rpm/microshift.spec
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/auto-rebase/assets_cert_manager.yaml
eslutsky
force-pushed
the
bump-cert-manager-to-19.0
branch
4 times, most recently
from
a522292 to
958f859
Compare
eslutsky
changed the title
openshift-ci
Bot
removed
the
needs-rebase
eslutsky
force-pushed
the
bump-cert-manager-to-19.0
branch
from
5d7634e to
65fa9ad
Compare
eslutsky
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@eslutsky: This pull request references USHIFT-6836 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
eslutsky
force-pushed
the
bump-cert-manager-to-19.0
branch
from
21882af to
9ee2a08
Compare
Contributor
Author
|
/retest |
eslutsky
force-pushed
the
bump-cert-manager-to-19.0
branch
from
9ee2a08 to
e7b218c
Compare
openshift-ci
Bot
added
needs-rebase
…v4.22 Signed-off-by: Evgeny Slutsky <eslutsky@redhat.com>
Signed-off-by: Evgeny Slutsky <eslutsky@redhat.com>
eslutsky
force-pushed
the
bump-cert-manager-to-19.0
branch
from
310175b to
48ecb24
Compare
Signed-off-by: Evgeny Slutsky <eslutsky@redhat.com>
eslutsky
force-pushed
the
bump-cert-manager-to-19.0
branch
from
48ecb24 to
137cd5a
Compare
Contributor
|
@eslutsky: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rebases the cert-manager operator from the v4.22 operator index (cert-manager-operator.v1.19.0) and adds TrustManager addon support.
Key changes: